Skip to content

nit: redundant call to sb.toString before print #2464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

geminicaprograms
Copy link
Contributor

Command line arguments chapter ends with the following code snippet:

  ...
  sb.toString
  println(sb)

and those two lines IMHO should be replaced with one as:

  • sb.toString performs actual string materialisation from buffer and returns it,
    but nothing is actually using it
  • println(sb) relies on StringBuilder having properly implemented toString
    method (which is obviously the case) and calls it implicitly again and prints
    it to the std out this time

I have proposed to reduce it to println(sb.toString) to give clear intent of toString
being actually used to produce the content that is outputted but println(sb) would
do to, WDYT?

'Command line arguments' chapter ends with the following code snippet:
  ...
  sb.toString
  println(sb)

and those two lines IMHO should be replaced with one as:
* 'sb.toString' performs actual string materialisation from buffer and returns it,
  but nothing is actually using it
* 'println(sb)' relies on StringBuilder having properly implemented 'toString'
  method (which is obviously the case) and calls it implicitly again and prints
  it to the std out this time

I have proposed to reduce it to 'println(sb.toString)' to give clear intent of 'toString'
being actually used to produce the content that is outputted but 'println(sb)' would
do to.
Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks!

@bishabosha bishabosha merged commit fe5f757 into scala:main Jul 7, 2022
@geminicaprograms geminicaprograms deleted the patch-1 branch July 7, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants